Skip to content

Conversation

@BYK
Copy link
Member

@BYK BYK commented Jun 18, 2025

Making the HTTP2 transport the default when h2 and httpcore packages are installed. We've been testing this on Sentry SaaS for a while without any issues.

We should promote installing the SDK as sentry-sdk[http2] for this to be picked up though. Since we still have to support Python 3.7 and h2 not being supported there, we cannot install it by default and use HTTP2 directly.


EDIT: please merge after potel dogfooding

@BYK BYK requested a review from a team as a code owner June 18, 2025 11:16
@BYK BYK marked this pull request as draft June 18, 2025 11:17
@codecov
Copy link

codecov bot commented Jun 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.85%. Comparing base (38cc2a8) to head (545bbe9).
⚠️ Report is 1 commits behind head on potel-base.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@              Coverage Diff               @@
##           potel-base    #4492      +/-   ##
==============================================
+ Coverage       84.75%   84.85%   +0.09%     
==============================================
  Files             158      158              
  Lines           16093    16093              
  Branches         2564     2564              
==============================================
+ Hits            13640    13656      +16     
+ Misses           1660     1648      -12     
+ Partials          793      789       -4     
Files with missing lines Coverage Δ
sentry_sdk/consts.py 99.06% <ø> (ø)
sentry_sdk/transport.py 83.90% <100.00%> (-0.21%) ⬇️

... and 3 files with indirect coverage changes

@BYK BYK marked this pull request as ready for review June 18, 2025 13:36
@BYK
Copy link
Member Author

BYK commented Jun 18, 2025

The test failures look unrelated and consistent with each other.

@sl0thentr0py
Copy link
Member

ty @BYK, will take over and fix the tests.

will also make this a draft till we're ready to merge!

@sl0thentr0py sl0thentr0py marked this pull request as draft June 18, 2025 14:12
@BYK BYK marked this pull request as ready for review September 2, 2025 12:36
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @BYK, this lgtm. Two things:

  • please check out the Cursor comment about still setting the option via experiments in one of the tests, afait this is actually an oversight
  • please add a note about this to the MIGRATION_GUIDE

I'll take some time this week to also add this to the migration guide in the docs, together with the async transport.

@BYK BYK merged commit bdca31d into potel-base Sep 3, 2025
125 checks passed
@BYK BYK deleted the byk/breaking/http2-default branch September 3, 2025 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants